-
Notifications
You must be signed in to change notification settings - Fork 997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test case naming clean up #3143
Conversation
def description(case_description: str): | ||
def entry(fn): | ||
return with_meta_tags({'description': case_description})(fn) | ||
return entry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not in use in this PR, but it can be utilized to add notes to the test vector results in future PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow this is amazing! nice work :)
I left a few comments but generally looks very good, thanks for doing this!
_new_altair_mods = { | ||
**{'sync_aggregate': [ | ||
'eth2spec.test.altair.block_processing.sync_aggregate.test_process_' + key | ||
for key in ['sync_aggregate', 'sync_aggregate_random'] | ||
]}, | ||
**{key: 'eth2spec.test.altair.block_processing.test_process_' + key for key in [ | ||
'deposit', | ||
]} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is outside the scope of this PR but it would be very nice to have some kind of linter that would take the spec and test code and ensure we didn't miss anything...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. 😭
And it would be nice to auto-generate the mods or with more clever settings.
tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_deposit.py
Outdated
Show resolved
Hide resolved
tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_deposit.py
Outdated
Show resolved
Hide resolved
tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_deposit.py
Outdated
Show resolved
Hide resolved
tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_deposit.py
Outdated
Show resolved
Hide resolved
@with_altair_and_later | ||
@spec_state_test | ||
@always_bls | ||
def test_ineffective_deposit_with_bad_fork_version_and(spec, state): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_ineffective_deposit_with_bad_fork_version_and
... and what? :)
tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_deposit.py
Show resolved
Hide resolved
) | ||
|
||
|
||
@with_phases([ALTAIR]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be "altair and later"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is the control group of bellatrix/block_processing/test_process_deposit.py::test_ineffective_deposit_with_previous_fork_version
. Altair's previous fork is exactly the correct GENESIS_FORK_VERSION
while others' are not.
Added more comments (34907d2) and current_fork_version
test case (e5709a5).
d11189b
to
338806d
Compare
…est_ineffective_deposit_with_bad_fork_version`
@ralexstokes thanks for the reviews! 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow! very nice.
Address #3118
description
spec test decoratorprocess_deposit
tests